Skip to content

Fix: Prevent Adapt.on('popup:opened') listener leak in openPopup (fixes #345) - #346

Open
oliverfoster wants to merge 1 commit into
masterfrom
issue/345
Open

Fix: Prevent Adapt.on('popup:opened') listener leak in openPopup (fixes #345)#346
oliverfoster wants to merge 1 commit into
masterfrom
issue/345

Conversation

@oliverfoster

Copy link
Copy Markdown
Member

Fixes #345

Fix

  • Bind the popup:opened handler once via this.listenTo(Adapt, ...) in preRender(), instead of calling Adapt.on() inside openPopup() on every invocation — Backbone unbinds it automatically when the view is removed
  • Only mark the active item visited when the opened popup is the one this narrative instance created, matched against the NotifyPopupView returned by notify.popup(), instead of reacting to any popup opened anywhere in the course

Testing

  1. Add a narrative component with items that have titles/straplines set, and preview the course
  2. Open and close a narrative item's popup several times; confirm Adapt._events['popup:opened'].length no longer grows
  3. While a narrative item is visible, trigger an unrelated popup elsewhere (e.g. another component's notify.popup()/alert) and confirm the narrative item's visited state is unaffected
  4. Confirm normal behaviour still works: opening a narrative item's own popup still marks that item visited

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Needs Reviewing

Development

Successfully merging this pull request may close these issues.

Adapt.on('popup:opened') listener leak in NarrativeView.openPopup

1 participant